Skip to content

Conversation

@aschackmull
Copy link
Contributor

This is a followup for #19885, which made this possible.

The concept of post-dominance may either assume normal termination of a callable, or try to account for exceptions. This PR switches Java to the former, which is in line with the shared CFG library. Indeed, trying to account for exceptions in post-dominance usually isn't what's desired, and it's also somewhat meaningless, since many potential exception edges are elided in the CFG if there's no enclosing try-block. As such, I'll consider this to be a bug-fix (and a somewhat minor one given that post-dominance is only rarely used).

@aschackmull aschackmull requested a review from a team as a code owner August 4, 2025 13:15
Copilot AI review requested due to automatic review settings August 4, 2025 13:15

This comment was marked as outdated.

@github-actions github-actions bot added the Java label Aug 4, 2025
@aschackmull
Copy link
Contributor Author

I've started dca with the only two queries using post-dominance: java/lazy-initialization (LazyInitStaticField.ql), java/improper-webview-certificate-validation (ImproperWebViewCertificateValidation.ql)

@aschackmull
Copy link
Contributor Author

Dca shows zero impact (as could be expected).

@aschackmull aschackmull requested a review from a team as a code owner August 5, 2025 06:58
@github-actions github-actions bot added the Kotlin label Aug 5, 2025
@aschackmull aschackmull force-pushed the java/postdom-normal branch from 85d7b7a to 273429d Compare August 5, 2025 08:33
Copilot AI review requested due to automatic review settings August 5, 2025 08:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies Java's post-dominance calculation to assume normal termination rather than accounting for exceptions. This aligns Java with the shared CFG library approach and fixes a bug where exception handling was inconsistent due to elided exception edges in the CFG when no try-block is present.

  • Changes post-dominance exit nodes from general ExitNode to NormalExitNode only
  • Updates test expectations to reflect the new post-dominance relationships

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll Changes post-dominance calculation to use normal exit nodes only
java/ql/test-kotlin1/library-tests/controlflow/basic/strictPostDominance.expected Updated test expectations for new post-dominance behavior
java/ql/test-kotlin2/library-tests/controlflow/basic/strictPostDominance.expected Updated test expectations for new post-dominance behavior

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Aug 5, 2025
@aschackmull aschackmull merged commit c59d20a into github:main Aug 5, 2025
17 of 18 checks passed
@aschackmull aschackmull deleted the java/postdom-normal branch August 5, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Kotlin no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants